-
Notifications
You must be signed in to change notification settings - Fork 131
multi: Add label to PublishTransaction
#1411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we should use
tapd-asset-transfer
tapd-asset-issuance
to align with our proof type terminology.
And maybe even?
TAP-transfer
TAP-issuance
I vote for:
|
Pull Request Test Coverage Report for Build 14731819540Details
💛 - Coveralls |
Can we go with one of the above ideas so we can merge this? |
@gijswijs, remember to re-request review from reviewers when ready |
822bf44
to
c19e61d
Compare
I've gone with:
I've also made the labels constants and rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🦺
This PR chanegs the label used for Txs published to the network. Before, all Txs got the label
tapd-asset-minting
. Now, the label depends on the context. Currently we use two labels:tapd-asset-minting
tapd-asset-send
In the future we could even allow for more granular information being passed to the label.